Skip to content

feat(ui5-bar): allow role change #11134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat(ui5-bar): allow role change #11134

wants to merge 4 commits into from

Conversation

tsanislavgatev
Copy link
Contributor

@tsanislavgatev tsanislavgatev commented Mar 19, 2025

Introduces a new accessibleRole property to the ui5-bar component, allowing developers to customize the ARIA role of the bar for accessibility purposes.

By default, the role is set to "toolbar", but it can now be explicitly changed by the application.

fixes: #10990

@tsanislavgatev tsanislavgatev requested a review from unazko March 20, 2025 14:14
*
*/
@property()
accessibleRole: `${BarAccessibleRole}` = "Toolbar";
Copy link
Contributor

@unazko unazko Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to provide recommendations to the users on how to use this new property.
In the openui5 project the logic is internal and no explanations are required.
Also in order to properly test this ourselves we could provide a dev sample that changes the number of interactive elements at runtime and therefore changes the role accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added additional documentation and example. But i've added a static example with just two bars explaining the both cases, because we don't provide dynamic change of the role, so such example wont be needed.

@unazko unazko requested a review from Stoev April 16, 2025 11:38
Copy link

@Stoev Stoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider adding short explanation:

feat(ui5-bar): allow role changes - maybe "role changes" is more explanatory?..
OR, eventually:
feat(ui5-bar): support for specific roles - just another suggestion...

And maybe consider adding some explanatory text, for example:

  • Added new property accessibleRole to the Bar class.
  • Bar component now supports custom roles for specific use cases.
  • Updated the accInfo method to include the effectiveRole.

@tsanislavgatev tsanislavgatev requested a review from Stoev April 16, 2025 14:59
@tsanislavgatev
Copy link
Contributor Author

tsanislavgatev commented Apr 16, 2025

Please consider adding short explanation:

feat(ui5-bar): allow role changes - maybe "role changes" is more explanatory?.. OR, eventually: feat(ui5-bar): support for specific roles - just another suggestion...

And maybe consider adding some explanatory text, for example:

  • Added new property accessibleRole to the Bar class.
  • Bar component now supports custom roles for specific use cases.
  • Updated the accInfo method to include the effectiveRole.

Hi @Stoev,

I was about to add you after I add the documentation that's ready for review. I've just added the documentation. The version you've reviewed is pre-documentation. Can you please review it now? I've added API documentation with example when and how it should be used. Also an additional sample was added showing both cases of the bar usage.

@tsanislavgatev tsanislavgatev requested a review from unazko April 16, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SF][Bar]: Role provided to bar is not cascaded to the DOM and instead take toolbar as default
3 participants